doc: Update capabilities readme to solve to open permissions in it #4469

dragoangel · 2024-03-03T10:55:06Z

Describe what this PR does

Update documentation about required capabilities by Ceph CSI plugin and improve examples:

limiting RBD to only one pool which only necessary to work with CSI
limit CephFS to only one fs, only allowing read access to volumes with write and snapshot capabilities to only used by CSI subvolume
removes unneeded complexity of splitting provisioner with controller from node capabilities, as they doesn't provide any security benefits at all, just confuse people.

Is there anything that requires special attention

N/A

Related issues

#1818
Fixes: #2687

Future concerns

N/A

Checklist:

Commit Message Formatting: Commit titles and messages follow
guidelines in the developer
guide.
Reviewed the developer guide on Submitting a Pull
Request
Pending release
notes
updated with breaking and/or notable changes for the next major release.
Documentation has been updated, if necessary.
Unit tests have been added, if necessary.
Integration tests have been added, if necessary.

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

/retest ci/centos/<job-name>: retest the <job-name> after unrelated
failure (please report the failure too!)

nixpanic · 2024-03-04T11:35:16Z

Thanks for the PR!

Please update the commit subject with doc: as prefix, and add your Signed-off-by: line. There should be a single commit (please squash them together) as well.

dragoangel · 2024-03-04T11:52:37Z

@nixpanic now it's good?

nixpanic · 2024-03-04T12:23:02Z

@nixpanic now it's good?

Thanks! Yes, I think it looks good. 2 reviews are needed to get it merged, maybe someone else has other comments.

nixpanic

Sorry, tests were not passing, you will need to update the PR once again 😞

nixpanic · 2024-03-07T10:06:12Z

docs/capabilities.md

 ```

 To get more insights on capabilities of CephFS you can refer
 [this document](https://ceph.readthedocs.io/en/latest/cephfs/client-auth/)

 ## Command to a create user with required capabilities

-`kubernetes` in the below commands represents an user which is subjected
-to change as per your requirement.
+`USER`, `POOL` and `FS_NAME` with `SUB_VOL` variables bellow is subject to change, please adjust them to your needs.


typo: bellow -> below

Please add a linebreak before 80 chars.

./docs/capabilities.md:40: MD013 Line length

Sure, okay 👍

@nixpanic can you please advice what about code blocks? Can they be longer? Like:
mds "allow r fsname=cephfs path=/volumes, allow rws fsname=cephfs path=/volumes/csi"

I fixed everything else, except that

Thanks! I think code blocks should be fine. CI runs have been approved again, so we'll know in a few minutes :)

Everything passed, looks good 👍

docs/capabilities.md

Pull request has been modified.

ceph-csi-bot · 2024-03-08T15:53:14Z

/test ci/centos/k8s-e2e-external-storage/1.29

ceph-csi-bot · 2024-03-08T15:53:15Z

/test ci/centos/upgrade-tests-cephfs

ceph-csi-bot · 2024-03-08T15:53:15Z

/test ci/centos/mini-e2e-helm/k8s-1.29

ceph-csi-bot · 2024-03-08T15:53:15Z

/test ci/centos/upgrade-tests-rbd

ceph-csi-bot · 2024-03-08T15:53:16Z

/test ci/centos/mini-e2e/k8s-1.29

ceph-csi-bot · 2024-03-08T15:53:16Z

/test ci/centos/k8s-e2e-external-storage/1.28

ceph-csi-bot · 2024-03-08T15:53:17Z

/test ci/centos/mini-e2e-helm/k8s-1.28

ceph-csi-bot · 2024-03-08T15:53:17Z

/test ci/centos/k8s-e2e-external-storage/1.27

ceph-csi-bot · 2024-03-08T15:53:17Z

/test ci/centos/mini-e2e/k8s-1.28

ceph-csi-bot · 2024-03-08T15:53:18Z

/test ci/centos/mini-e2e-helm/k8s-1.27

ceph-csi-bot · 2024-03-08T15:53:18Z

/test ci/centos/mini-e2e/k8s-1.27

Madhu-1 · 2024-03-12T11:36:05Z

docs/capabilities.md

-mds 'allow rw'
+USER=csi-cephfs
+FS_NAME=cephfs
+SUB_VOL=csi


this will always be csi we dont allow changing it

Actually this not true, it possible to change it to another subvolume, and I used it this way already to host multiply clusters on same cehpfs but on different subvolumes.

Madhu-1 · 2024-03-12T11:39:35Z

docs/capabilities.md

-mon 'profile rbd' \
-osd 'profile rbd' \
-mgr 'allow rw'
+USER=csi


USER=csi to USER=csi-rbd lets keep the names constant for both cephfs and rbd

@Madhu-1 the point that ceph-csi-rbd and ceph-csi-cephfs is different deployments that can (and should) have different users that each has own permissions, I don't understand why they should be one mixed super overpowered thing. The point of this PR is to provide example of real permissions needed and not overgive permissions where it doesn't needed.

i dont see any point in calling rbd user name as just csi user and cephfs as csi-cephfs, ideally both should be similar in the documented examples. but as its configurable users can configure it as per the requirement.

I can change it to csi-rbd to align with csi-cephfs, so they will be definitely not same, but will look better, is this okay?

IMO yes this looks ok.

Pushed, please review 😊

Pull request has been modified.

nixpanic · 2024-03-13T14:12:54Z

@Mergifyio queue

mergify · 2024-03-13T14:13:22Z

queue

🛑 The pull request has been removed from the queue `default`

The pull request can't be updated.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

nixpanic · 2024-03-13T16:59:31Z

@Mergifyio rebase

Signed-off-by: Dmytro Alieksieiev <1865999+dragoangel@users.noreply.github.com>

mergify · 2024-03-13T16:59:59Z

rebase

✅ Branch has been successfully rebased

nixpanic · 2024-03-13T17:00:03Z

@Mergifyio queue

mergify · 2024-03-13T17:01:18Z

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 6c43789

ceph-csi-bot · 2024-03-13T17:01:43Z

/test ci/centos/k8s-e2e-external-storage/1.27

ceph-csi-bot · 2024-03-13T17:01:43Z

/test ci/centos/mini-e2e-helm/k8s-1.27

ceph-csi-bot · 2024-03-13T17:01:44Z

/test ci/centos/k8s-e2e-external-storage/1.29

ceph-csi-bot · 2024-03-13T17:01:44Z

/test ci/centos/k8s-e2e-external-storage/1.28

ceph-csi-bot · 2024-03-13T17:01:44Z

/test ci/centos/mini-e2e/k8s-1.27

ceph-csi-bot · 2024-03-13T17:01:44Z

/test ci/centos/upgrade-tests-cephfs

ceph-csi-bot · 2024-03-13T17:01:45Z

/test ci/centos/mini-e2e-helm/k8s-1.29

ceph-csi-bot · 2024-03-13T17:01:45Z

/test ci/centos/mini-e2e-helm/k8s-1.28

ceph-csi-bot · 2024-03-13T17:01:45Z

/test ci/centos/upgrade-tests-rbd

ceph-csi-bot · 2024-03-13T17:01:45Z

/test ci/centos/mini-e2e/k8s-1.29

ceph-csi-bot · 2024-03-13T17:01:45Z

/test ci/centos/mini-e2e/k8s-1.28

dragoangel changed the title ~~Update capabilities readme to solve misleading information in it~~ Update capabilities readme to solve to open permissions in it Mar 3, 2024

nixpanic changed the title ~~Update capabilities readme to solve to open permissions in it~~ doc: Update capabilities readme to solve to open permissions in it Mar 4, 2024

mergify bot added ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures component/docs Issues and PRs related to documentation labels Mar 4, 2024

dragoangel force-pushed the patch-1 branch from ad183d8 to 82fa996 Compare March 4, 2024 11:52

nixpanic previously approved these changes Mar 4, 2024

View reviewed changes

nixpanic requested a review from a team March 4, 2024 12:21

nixpanic reviewed Mar 7, 2024

View reviewed changes

dragoangel force-pushed the patch-1 branch from 82fa996 to c9d23c5 Compare March 8, 2024 08:25

nixpanic previously approved these changes Mar 8, 2024

View reviewed changes

nixpanic requested a review from a team March 8, 2024 14:27

nixpanic added the ok-to-test Label to trigger E2E tests label Mar 8, 2024

ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Mar 8, 2024

Madhu-1 reviewed Mar 12, 2024

View reviewed changes

dragoangel force-pushed the patch-1 branch from c9d23c5 to b0246ea Compare March 12, 2024 11:54

dragoangel requested review from Madhu-1 and nixpanic March 12, 2024 18:13

Madhu-1 approved these changes Mar 13, 2024

View reviewed changes

nixpanic approved these changes Mar 13, 2024

View reviewed changes

doc: Update capabilities readme to solve to open permissions

064c3fb

Signed-off-by: Dmytro Alieksieiev <1865999+dragoangel@users.noreply.github.com>

nixpanic force-pushed the patch-1 branch from b0246ea to 064c3fb Compare March 13, 2024 17:00

mergify bot added the ok-to-test Label to trigger E2E tests label Mar 13, 2024

ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Mar 13, 2024

mergify bot merged commit 6c43789 into ceph:devel Mar 13, 2024
32 of 34 checks passed

doc: Update capabilities readme to solve to open permissions in it #4469

doc: Update capabilities readme to solve to open permissions in it #4469

Conversation

dragoangel commented Mar 3, 2024 • edited Loading

Describe what this PR does

Is there anything that requires special attention

Related issues

Future concerns

nixpanic commented Mar 4, 2024

dragoangel commented Mar 4, 2024

nixpanic commented Mar 4, 2024

nixpanic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dragoangel Mar 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ceph-csi-bot commented Mar 8, 2024

ceph-csi-bot commented Mar 8, 2024

ceph-csi-bot commented Mar 8, 2024

ceph-csi-bot commented Mar 8, 2024

ceph-csi-bot commented Mar 8, 2024

ceph-csi-bot commented Mar 8, 2024

ceph-csi-bot commented Mar 8, 2024

ceph-csi-bot commented Mar 8, 2024

ceph-csi-bot commented Mar 8, 2024

ceph-csi-bot commented Mar 8, 2024

ceph-csi-bot commented Mar 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dragoangel Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dragoangel Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nixpanic commented Mar 13, 2024

mergify bot commented Mar 13, 2024 • edited Loading

🛑 The pull request has been removed from the queue default

nixpanic commented Mar 13, 2024

mergify bot commented Mar 13, 2024

✅ Branch has been successfully rebased

nixpanic commented Mar 13, 2024

mergify bot commented Mar 13, 2024 • edited Loading

✅ The pull request has been merged automatically

ceph-csi-bot commented Mar 13, 2024

ceph-csi-bot commented Mar 13, 2024

ceph-csi-bot commented Mar 13, 2024

ceph-csi-bot commented Mar 13, 2024

ceph-csi-bot commented Mar 13, 2024

ceph-csi-bot commented Mar 13, 2024

ceph-csi-bot commented Mar 13, 2024

ceph-csi-bot commented Mar 13, 2024

ceph-csi-bot commented Mar 13, 2024

ceph-csi-bot commented Mar 13, 2024

ceph-csi-bot commented Mar 13, 2024

dragoangel commented Mar 3, 2024 •

edited

Loading

dragoangel Mar 8, 2024 •

edited

Loading

dragoangel Mar 12, 2024 •

edited

Loading

dragoangel Mar 12, 2024 •

edited

Loading

mergify bot commented Mar 13, 2024 •

edited

Loading

🛑 The pull request has been removed from the queue `default`

mergify bot commented Mar 13, 2024 •

edited

Loading